Pull request for advanced metadata searches on hardcoded DID columns: https://github.com/rucio/rucio/pull/4746
Next step is to port this functionality to JSON metadata.
rmb@ubuntu:~/rucio$ docker-compose --file etc/docker/dev/docker-compose-storage.yml up -d
rmb@ubuntu:~/rucio$ docker exec -it dev_rucio_1 /bin/bash
[root@7b1b9528abbc rucio]# python3 -m pip uninstall -y sqlalchemy && python3 -m pip install sqlalchemy && tools/run_tests_docker.sh -air
Installed on fresh ubuntu VM following guide at https://rucio.readthedocs.io/en/latest/demo_environment.html.
Hit a problem with using docker-compose files:
rmb@ubuntu:~/rucio$ docker-compose --file etc/docker/dev/docker-compose-storage.yml up -d
ERROR: The Compose file './etc/docker/dev/docker-compose-storage.yml' is invalid because:
Unsupported config option for services: 'graphite'
that needed a version specification, i.e.
version: "3.5"
services:
Then another problem with running the testing suite:
[root@fbbf3dd8e28b rucio]# tools/run_tests_docker.sh
TypeError: _add_hint() missing 1 required positional argument: 'execution_options'
Failed to reset the database!
originating from tools/reset_database.py script:
[root@fbbf3dd8e28b tools]# python3 reset_database.py
TypeError: _add_hint() missing 1 required positional argument: 'execution_options'
Failed to reset the database!
figured it was some issue with sqlalchemy, so checked the version installed:
import sqlslchemy
print(sqlalchemy.__version__)
1.3.17
which is not the version specified in the requirements.txt (SQLAlchemy~=1.4.17). Not sure why it's installed a different minor version as ~= operator should only install 1.4.x? Anyway, uninstalled and installed:
[root@fbbf3dd8e28b rucio]# python3 -m pip uninstall sqlalchemy
Found existing installation: SQLAlchemy 1.3.17
Uninstalling SQLAlchemy-1.3.17:
Would remove:
/usr/local/lib64/python3.6/site-packages/SQLAlchemy-1.3.17.dist-info/*
/usr/local/lib64/python3.6/site-packages/sqlalchemy/*
Proceed (y/n)? y
Successfully uninstalled SQLAlchemy-1.3.17
WARNING: Running pip as root will break packages and permissions. You should install packages reliably by using venv: https://pip.pypa.io/warnings/venv
[root@fbbf3dd8e28b rucio]# python3 -m pip install sqlalchemy
Collecting sqlalchemy
Downloading SQLAlchemy-1.4.19-cp36-cp36m-manylinux_2_5_x86_64.manylinux1_x86_64.manylinux_2_17_x86_64.manylinux2014_x86_64.whl (1.5 MB)
|################################| 1.5 MB 1.7 MB/s
Requirement already satisfied: importlib-metadata in /usr/local/lib/python3.6/site-packages (from sqlalchemy) (3.10.1)
Collecting greenlet!=0.4.17
Downloading greenlet-1.1.0-cp36-cp36m-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (155 kB)
|################################| 155 kB 15.4 MB/s
Requirement already satisfied: zipp>=0.5 in /usr/local/lib/python3.6/site-packages (from importlib-metadata->sqlalchemy) (3.4.1)
Requirement already satisfied: typing-extensions>=3.6.4 in /usr/local/lib/python3.6/site-packages (from importlib-metadata->sqlalchemy) (3.10.0.0)
Installing collected packages: greenlet, sqlalchemy
Successfully installed greenlet-1.1.0 sqlalchemy-1.4.19
and reran problematic database init script:
[root@fbbf3dd8e28b tools]# python3 reset_database.py
Schema set in config, trying to create schema: dev
INFO [alembic.runtime.migration] Context impl PostgresqlImpl.
INFO [alembic.runtime.migration] Will assume transactional DDL.
INFO [alembic.runtime.migration] Running stamp_revision -> 30fa38b6434e
which now works. Checked database dids/replicas/rses table, which appears to be populated with sane looking entries.
Ran tools/run_docker_tests.sh:
[root@fbbf3dd8e28b rucio]# tools/run_tests_docker.sh
804 passed, 313 skipped, 11 xfailed, 501 warnings in 456.70s (0:07:36)
Can access the API inside the dev container with, e.g.:
curl https://localhost:443/ping --insecure
{"version":"1.25.3"}
The api endpoints are served through wsgi via httpd, see: https://github.com/rucio/containers/blob/master/server/rucio.conf.j2 https://github.com/rucio/rucio/tree/master/lib/rucio/web/rest/flaskapi/v1
The sequence of calls is something like:
CLI <-> /client <-> /web/flaskapi/v1 <-> /api <-> /core
Working from existing PR: https://github.com/rucio/rucio/pull/4139/files
Alterations as follows.
Replaces filter string to support OR on name filter.
Replaces "account" and "scope" in filter string with internal representation, although doesn't seem to replace as it uses insert? (https://github.com/rucio/rucio/pull/4139/files#diff-5ea528d2d3cdc9c1485149b48a8a333e4312d4dd8d91acf002820b52695ee910R72)
Replaces filter string to support OR on type filter, taken as a separate variable from the function input.
Filters can be either a dict or string, presumably depending on whether it's called from the CLI or programatically imported?
Some additional conversion of dates to strings if they come through in the dict.
Duplication of above logic re: type and dates.
Removal of logic adding filtering to query. This is now handled by passing the filter and base query to the inequality engine:
queries = inequality_engine(filters).createQueries(query_master=query)
Altered list_dids function, which is called by list_dids_extended in lib/rucio/core/did.py. Changes similar to those made to list_dids function in lib/rucio/core/did.py.
The engine itself.
Syntax (= to == on type).
Not sure why, but adding UUID to scope name.
Doesn't work. Get this:
[root@00fd1e999d57 bin]# rucio list-dids test:*
2021-06-25 10:58:05,286 ERROR The resource doesn't support the requested operation.
Details: Valid type are: ['all', 'collection', 'container', 'dataset', 'file']
switching to master to get the prevous, printing out url/payload (ln82, didclient.py):
[root@00fd1e999d57 bin]# rucio list-dids test:*
https://rucio:443/dids/test/dids/search?name=%2A&long=True&type=collection&recursive=False {'name': '*', 'long': True, 'type': 'collection', 'recursive': False}
+----------------+--------------+
| SCOPE:NAME | [DID TYPE] |
|----------------+--------------|
| test:container | CONTAINER |
| test:dataset1 | DATASET |
| test:dataset2 | DATASET |
| test:dataset3 | DATASET |
+----------------+--------------+
(where %2A is an url encoded asterisk), i.e. name=*
back to metadata branch, the above print command gives:
https://rucio:443/dids/test/dids/search?filters=%2C+name+%3D%3D+%2A&long=True&recursive=False {'filters': ', name == *', 'long': True, 'recursive': False}
i.e. filters=,name==*
which doesn't look right.
Traced this through to logic in the rucio executable. Doesn't cater for unspecified filter clauses, so replaced with:
if name:
if filters:
if ';' in filters: # filter contains multiple OR'ed clauses
filters.replace(';', ', name == {};'.format(name)) # -> add filter for name to each OR clause
else: # filter contains only single OR clause
filters += ', name == {}'.format(name) # -> add filter for name at end
else: # no filter specified
filters += 'name == {}'.format(name) # -> add name as single argument to filter
Now doesn't look like the type parameter is being passed through the flaskapi. Looks like they were using webpy when this PR was submitted, so will have to add new logic to web/rest/flaskapi/v1/dids.py Search endpoint:
2021-06-28 16:01:33,737 ERROR The resource doesn't support the requested operation.
Details: Valid type are: ['all', 'collection', 'container', 'dataset', 'file']
Issue is with type being Nonetype in core/did.py list_dids function, although should be assigned from the function input as it's now not being passed if it's Nonetype in the flask api. Added clause to check this.
Works. Or rather, it runs, and the basic functionality appears to be there.
[root@3dc3e2650363 rucio]# echo 'flush_all' | nc localhost 11211 && httpd -k graceful && rucio list-dids test:* --filter="name==*"
OK
[Wed Jun 30 15:59:31.147275 2021] [proxy:info] [pid 17152:tid 140171895343232] AH01145: Sharing worker 'https://localhost' instead of creating new worker 'https://localhost'
AH00558: httpd: Could not reliably determine the server's fully qualified domain name, using 172.23.0.2. Set the 'ServerName' directive globally to suppress this message
+----------------------------------------------------------------------+-------------------+
| SCOPE:NAME | [DID TYPE] |
|----------------------------------------------------------------------+-------------------|
| test:container | DIDType.CONTAINER |
| test:dataset1 | DIDType.DATASET |
| test:dataset2 | DIDType.DATASET |
| test:dataset3 | DIDType.DATASET |
| test:tests.rucio_client_test_server_e681e0b9eb3e4dd5b5179b1d968e617b | DIDType.DATASET |
+----------------------------------------------------------------------+-------------------+
[root@3dc3e2650363 rucio]# echo 'flush_all' | nc localhost 11211 && httpd -k graceful && rucio list-dids test:* --filter="name==dataset*"
OK
[Wed Jun 30 16:00:09.990434 2021] [proxy:info] [pid 17730:tid 140085079107712] AH01145: Sharing worker 'https://localhost' instead of creating new worker 'https://localhost'
AH00558: httpd: Could not reliably determine the server's fully qualified domain name, using 172.23.0.2. Set the 'ServerName' directive globally to suppress this message
+---------------+-----------------+
| SCOPE:NAME | [DID TYPE] |
|---------------+-----------------|
| test:dataset1 | DIDType.DATASET |
| test:dataset2 | DIDType.DATASET |
| test:dataset3 | DIDType.DATASET |
+---------------+-----------------+
[root@3dc3e2650363 rucio]# echo 'flush_all' | nc localhost 11211 && httpd -k graceful && rucio list-dids test:* --filter="name==dataset*, name==container"
OK
[Wed Jun 30 16:00:39.740134 2021] [proxy:info] [pid 18308:tid 140118206601344] AH01145: Sharing worker 'https://localhost' instead of creating new worker 'https://localhost'
AH00558: httpd: Could not reliably determine the server's fully qualified domain name, using 172.23.0.2. Set the 'ServerName' directive globally to suppress this message
+--------------+--------------+
| SCOPE:NAME | [DID TYPE] |
|--------------+--------------|
+--------------+--------------+
[root@3dc3e2650363 rucio]# echo 'flush_all' | nc localhost 11211 && httpd -k graceful && rucio list-dids test:* --filter="name==dataset*; name==container"
OK
[Wed Jun 30 16:00:57.985338 2021] [proxy:info] [pid 18886:tid 140675732019328] AH01145: Sharing worker 'https://localhost' instead of creating new worker 'https://localhost'
AH00558: httpd: Could not reliably determine the server's fully qualified domain name, using 172.23.0.2. Set the 'ServerName' directive globally to suppress this message
+----------------+-------------------+
| SCOPE:NAME | [DID TYPE] |
|----------------+-------------------|
| test:dataset1 | DIDType.DATASET |
| test:dataset2 | DIDType.DATASET |
| test:dataset3 | DIDType.DATASET |
| test:container | DIDType.CONTAINER |
+----------------+-------------------+
Tests fail on TestInequalityEngineOnline::test_InequalityEngineRetrocompatibilityLength, which is alluded to in the PR comments.
[root@3dc3e2650363 rucio]# tools/ .sh test_inequality_engine.py
=============================================================== short test summary info ================================================================
FAILED test_inequality_engine.py::TestInequalityEngineOnline::test_InequalityEngineRetrocompatibilityLength - AssertionError: 0 != 1
====================================================== 1 failed, 29 passed, 348 warnings in 8.21s ======================================================
If altering server side stuff, e.g. the api or REST interface, need to reset the memcache and httpd server on change, i.e.
echo 'flush_all' | nc localhost 11211 && httpd -k graceful && rucio list-dids test:*
Individual tests from single modules can be run like:
echo 'flush_all' | nc localhost 11211 && httpd -k graceful && tools/pytest.sh test_did.py -k "test_list_dids"
If running the test suite twice, omit the alembic upgrade/downgrade, otherwise you get an error:
tools/run_tests_docker.sh -a
Transfers between storage can be included using the additional -r flag. If you just want to initialise the environment, use the -i flag.
Currently there is a util function parse_did_filter_from_string that handles the filter input. This PR doesn't use this, and instead leaves the parsing to the
inequality engine. IMO, some form of this would be best kept near the client, so decided on refactoring to move interpretation of the filter string in an equivalent of this
function parse_did_filter_from_string_fe. This has the added advantage that the filter variable is always expected to be a single type as it
progresses through the didclient -> rest interface -> api -> core, which it isn't currently, with ugly switch/case logic to handle strings/dicts. This makes
it confusing to follow. Also minimises changes required further along the workflow (didclient, rest, api).
Note, the type variable can be set at the end of the chain in core, so this assignment as a filter is left to the core (in case of some internal calls to
core list_dids).
Had some issues with the database not being contactable, this can be resolved by restarting all the services.
Testing ranged metadata search:
[root@7b1b9528abbc rucio]# rucio list-dids test:* --filter="name=dataset*"
+---------------+-----------------+
| SCOPE:NAME | [DID TYPE] |
|---------------+-----------------|
| test:dataset1 | DIDType.DATASET |
| test:dataset2 | DIDType.DATASET |
| test:dataset3 | DIDType.DATASET |
+---------------+-----------------+
[root@7b1b9528abbc rucio]# rucio get-metadata test:dataset1 | grep created_at
created_at: 2021-07-20 09:49:09
[root@7b1b9528abbc rucio]# rucio get-metadata test:dataset2 | grep created_at
created_at: 2021-07-20 09:49:10
[root@7b1b9528abbc rucio]# rucio get-metadata test:dataset3 | grep created_at
created_at: 2021-07-20 09:49:13
[root@7b1b9528abbc rucio]# rucio list-dids test:* --filter="name=dataset*, created_at>2021-07-20T09:49:10, created_at<2021-07-20 09:49:13"
+---------------+-----------------+
| SCOPE:NAME | [DID TYPE] |
|---------------+-----------------|
| test:dataset2 | DIDType.DATASET |
+---------------+-----------------+
[root@7b1b9528abbc rucio]# rucio list-dids test:* --filter="name=dataset*, created_at<2021-07-20 09:49:10; name=dataset*, created_at>2021-07-20 09:49:11"
+---------------+-----------------+
| SCOPE:NAME | [DID TYPE] |
|---------------+-----------------|
| test:dataset1 | DIDType.DATASET |
| test:dataset3 | DIDType.DATASET |
+---------------+-----------------+
Bug in did_column_meta.py, no comma between name and length HARDCODED_KEYS means they got concatenated. When trying to set_metadata, it couldn't find the
"length" key and was instead finding "namelength".
Original PR hasn't taken into account mutual files in OR returns. Have had to keep track of this in core/did.py (ignore_dids).
These also needed refactoring. Can run individual module with:
[root@327a11a1796b rucio]# tools/pytest.sh test_inequality_engine.py
All currently pass.
[root@327a11a1796b rucio]# tools/run_tests_docker.sh
824 passed, 313 skipped, 11 xfailed, 501 warnings in 474.37s (0:07:54)
First, see the commit guide: https://rucio.readthedocs.io/en/latest/contributing.html
Make sure personal access tokens, <PAT>, are set in the git config:
git config --global user.name "Rob Barnsley"
git config --global user.email "rob.barnsley@skao.int"
Configure git commit messages:
./tools/configure_git.sh
Make sure the upstream is set in .git/config to rucio/rucio:
[remote "upstream"]
url = https://<USERNAME>:<PAT>@github.com/rucio/rucio.git
Change to master branch and fetch:
git checkout master
git fetch --all
Squash your commits by rebasing:
git checkout <feature_branch>
git rebase -i HEAD~<n_commits>
where <n_commits> is the number of commits to squash (can be obtained by git log).
When the first vim editor window box pops up, change the "pick" attribute of whichever commits you want to merge to "squash".
Upon closing, another vim editor window will open allowing you to change the commit messages. By default, it will keep all the commit messages. Change it so the commit message is of the form
"<component>: <change_message> #<issue number>".
Next, check that upstream changes aren't conflicting with your local feature branch, can do this nondestructively via:
git checkout master
git pull upstream master
git merge <feature_branch> --no-ff --no-commit
This will yield a list of conflicts. After you've finished conflict resolution stash the changes:
git stash save
which will abort the merge. Then pop the changes back into your local working directory:
git stash pop
The merge can be aborted at any time without stashing via:
git merge --abort
Rename your branch (if you didn't create it initially via ./tools/create_*_branch):
git branch -m <new_branch_name>
branch names should be of the form:
Currently failing for some builds.
Invoked locally by:
rmb@ubuntu:~/rucio$ ./tools/run_autotests.sh
Can spawn a single autotest dockerised environment by first editing /etc/docker/test/matrix.yml to isolate the particular os/python/database combination and test type (client, client_syntax, doc, all).
This needs to be followed up by editing /etc/tools/test/run_test.py, and depending on whether you're running syntax checks (which do not require HTTPD) or "all", alter the
corresponding run function (run_test_directly & run_with_httpd respectively). For the former, you need to comment out the call to the run function on ln246 and instead
print this command to the terminal so it can be manually invoked. For the latter, you can add something like:
import time
time.sleep(36000)
on ln306 which keeps the container running.
Beware that if you run binaries or use library functions then the path that is being referenced is /usr/local/bin/ and /usr/local/lib respectively. Test logic is however taken
from the distribution in /opt.
Encountered some mirror problems in the container, can't run with centos distros:
14: curl#6 - “Could not resolve host: mirrorlist.centos.org 8; Unknown error”
Answer is to restart the docker service or, failing that, add --network=host to the args parameter in build_images in /tools/test/build_images.py.
'%a, %d %b %Y %H:%M:%S UTC') and hadn't allowed for in the engine.voname in rucio cfg maps via get_vo() function to a short name in the vo database table. Hadn't accounted for this.add_did() function seems to have different "type_" field rather than "type". Someone seems to have pushed breaking changes into master?